-
-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use roxy.shinylive #775
use roxy.shinylive #775
Conversation
Code Coverage Summary
Diff against main
Results for commit: ed824b0 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 22 suites 13m 14s ⏱️ Results for commit ed824b0. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Results for commit 10237b3 ♻️ This comment has been updated with latest results. |
Signed-off-by: Pawel Rucki <[email protected]>
The idea of adding a shinylive session for examples is pretty cool, thanks for showcasing this! I like the idea that users can see the examples in a Shiny context right away when they read our online documentation and I think this is a valid use case for a shiny oriented R package and can accelerate learning process. Here are couple thoughts as I play around with this: The edited .Rd fileThe portion being added to the .Rd file looks very straightforward. However, I'm wondering what would happen if the examples are complex or if we have a lot of Shiny app examples for one function. Could this result in a bloated .Rd file that is no longer lightweight? If so, is there any risk that we should consider? I also tried to see how the documentation would look when opened locally, i.e. PS: Not sure if this happened to you, but once when I ran shinylive teal app examplesAfter running See my attempt to click perform these actions: Since we're using an iframe solution, I'm not sure how easy it would be to troubleshoot this. I started to google around and so far I have the impression that it might be related to the CSS/JavaScript cross between the webpage in the iframe and the main page. If we can't find a good solution to this, my only concern is that users might not have the best experience exploring teal examples within the iframe boundaries. Maybe that's okay since you're also allowing users to try the example directly on the shinylive.io website. Overall, I think this is promising! |
Thanks for your comments - please find answers below
I somewhat anticipated this in the whole design and I made it as an opt-in - one needs to include something in order for this section to appear in the docs. So if an example is really complex then one can choose not to have it in shinylive by not including
This allows us to include part of examples:
I think it's a valid use case and I included it in the documentation here
In terms of size it's pretty lightweight. Looking into a random file in tmg it grows by exactly 9 lines.
Funnily - I have started with just a link and then extended this by iframe. I think it would be nice to include this somehow but the challenge is that the available space might not be enough.
Point taken. I can see that 125% width might not be a good call. Let me experiment with this more.
Hmmm that might be an evidence of a bad JS on our side. I have a few similar findings and I expect more to come. But that's actually a good thing as this would allow us to detect more bugs on our side.
Point taken. Let me have a few tries to improve this. If it cannot be done correctly I'm fine to come back to the solution without iframe. No pressure on this |
Hey @donyunardi please have a look now. I have pushed a few changes including visibility, width and fixing all the NOTES that I haven't noticed earlier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update.
I found couple more things:
z-index
I discovered why I couldn't interact with the filter panel: the iframe's z-index was lower than the navigation links.
After increasing the z-index, I was able to interact with the filter panel, as shown here:
However, increasing the z-index causes the navigation links to be placed behind the iframe, which might prevent users from seeing them when playing with the app. Personally, I think that's okay, but let me know what you think.
No Data in the teal app
Some teal app does not have any data:
If I click to the shinylive.io link on the page, I see this error message:
The error is:
Cannot read properties of null (reading 'deps')
Do you get the same problem?
Signed-off-by: Pawel Rucki <[email protected]>
Great suggestion! Thank you! Implemented in insightsengineering/roxy.shinylive@1181b1e and here in c5cc45f
Known issue. I reported this in posit-dev/r-shinylive#128. It's already fixed but not yet fully deployed. I would suggest to go with this as is and wait for a deploy of the fixed version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, I think this looks great!
test with insightsengineering/roxy.shinylive#1
Implementation of
roxy.shinylive
.Please run
pkgdown::build_site()
to see the difference.Extra:
teal.widgets
so that the example code is simpler@examples
with@examplesIf
. This is for our incorrect implementation of soft dependencies